Repository navigation
ci: run the milestone check on the light runner - #8008
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feb21a8119
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The milestone check is a few lines of shell that never checks out the PR, yet it sat queued for GitHub-hosted ubuntu-latest capacity. Route it through DASH_LIGHT_RUNNER like the other short metadata jobs, keeping ubuntu-latest as the fallback where the variable is unset. Variables are not passed to pull_request runs from forks, so trigger on pull_request_target, as the PR title check already does. The job reads only the milestone from the event payload, so running the base branch's workflow with base context is safe. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
feb21a8 to
3b6f9e7
Compare
PastaPastaPasta
changed the title
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the primary change: running the milestone check on the light runner. |
| Description check | |
The description directly explains the workflow changes, their purpose, testing, and expected behavior after merge. |
| Docstring Coverage | |
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
|
✅ Final review complete — no blockers (commit 3b6f9e7) · triage: low |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the complete diff at 3b6f9e7; no actionable issues found. The milestone job follows the existing light-runner selection pattern and safely uses the trusted base-branch workflow without checking out PR code, while preserving empty token permissions and quoted handling of milestone metadata. Current CI shows lint and Linux build/tests passing with nine build/dependency jobs pending and no failures; the revised runner selection requires post-merge confirmation because pull_request_target executes the base-branch workflow.
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The two-line CI change alters runner selection and the event trigger, but is small, contained, and straightforward to verify for a metadata-only job that does not execute PR code. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 38% left, weekly 57% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort medium); agentphase2-reviewer
thepastaclaw
added
the
pastaclaw:approved
…y so depends caches stop getting evicted ec0df91 ci: cap ccache at 300M and evict entries unused for a day (pasta) Pull request description: ## Issue being fixed or feature implemented PR builds are failing with "Depends cache restore missed and no built depends artifact was provided" (e.g. run 41252515931 on #8008: `aarch64-linux-build`, `linux64_tsan-build`, `linux64_multiprocess-build`). `Check cache` saw the depends entry at 23:27, the builds tried to restore it ~90 minutes later, and by then GitHub's LRU eviction had removed it. The eviction pressure is ccache. Every develop push saves one ccache entry per target, each sitting at the 600M cap: about 4.5 GB per push. Ten develop pushes landed in three hours, the runs overlapped, and `prune-ccache` only runs once all ten `src-*` jobs of a run are done. Between 22:32 and 00:12 nothing was pruned while ~14 GB of new entries were written against the 10 GB repository limit. Most of each 600M entry is objects from older commits. One build's working set is 0.1–0.25 GB per target: that's where cold v24.0.x builds end up, and where develop entries end up after a reset (multiprocess 182 MB, nowallet 220 MB, win64 241 MB). ## What was done? - Lower `CCACHE_MAXSIZE` from 600M to 300M. - Run `ccache --evict-older-than 1d` before the final cleanup. ccache refreshes a file's mtime on every hit, so this drops exactly the objects no build has used in a day, and the saved entry stays close to one build's working set. #7059 raised the cap from 400M to 600M because linux64/linux64_sqlite were thrashing at 400M. That was before the remote ccache: today an object evicted mid-build is fetched from the remote instead of recompiled. On recent develop pushes the remote served all but 1–2 of the local misses. ## How Has This Been Tested? - Ran ccache 4.12 locally: compiled two files, backdated both entries by two days, hit one again, then ran `--evict-older-than 1d`. The re-hit entry survived and the stale one was evicted (miss on rebuild). - CI on this PR does not exercise the change: `pull_request_target` runs the base branch's workflow, and ccache is only saved on develop pushes. The effect shows up in the `ccache` step summary and `gh cache list` sizes after merge. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(if you have permission to set one)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: cf2cec9b34ddcc75c437616c23c0a7cbe514f5e0016ebe5862b88673748682aead2bb54a558d93a2fc7244510531d4b7cda9a488a27002a8137c131f69f69ea8
thepastaclaw
removed
the
pastaclaw:approved
Issue being fixed or feature implemented
The "Require a milestone" check (
check-milestone.yml) sits queued waiting for GitHub-hostedubuntu-latestcapacity, even though it only runs a few lines of shell and never checks out the PR. The other short metadata jobs (PR title validation, merge-conflict labeling, conflict prediction, merge-check) already run on thedash-x64-lightpool via theDASH_LIGHT_RUNNERorg variable (#7900, #7902).What was done?
runs-onis now${{ vars.DASH_LIGHT_RUNNER || 'ubuntu-latest' }}, the same expression the other light jobs use.pull_request_targetinstead ofpull_request. Configuration variables are not passed topull_requestruns from forks, so with the old trigger the variable is empty for nearly every PR here and the job falls back toubuntu-latest. The first revision of this PR showed exactly that: its own milestone job still requestedubuntu-latest.semantic-pull-request.ymlalready usespull_request_targetfor the same kind of metadata check. This job reads onlygithub.event.pull_request.milestone.titleand.draftfrom the event, never checks out PR code, and haspermissions: {}. The activity types (milestoned,demilestoned,ready_for_review, …) are the same for both triggers.prevent-master-pr.ymlandclang-diff-format.ymlare left alone. Underpull_request_targetthe former would run master's copy of the workflow. The latter checks out and runs PR code.How Has This Been Tested?
zizmor 1.30.1 (the version CI lint uses) reports no findings with
.github/zizmor.yml.pull_request_targetruns the base branch's workflow, so this PR can't exercise its own change. After merge, the "Require a milestone" job on the next PR should show thedash-x64-lightlabel.Breaking Changes
None. The check name ("Require a milestone") is unchanged.
Checklist:
🤖 Generated with Claude Code